Clear ChunkedBitSet without reallocating
#147623
Merged
+2
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
This @bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit 279a5b3 with merge 9d4cb0c… To cancel the try build, run the command Workflow: https://github.com/rust-lang/rust/actions/runs/18453583260 |
rust-bors bot
added a commit
that referenced
this pull request
Oct 13, 2025
Clear `ChunkedBitSet` without reallocating
rustbot
added
the
S-waiting-on-perf
nnethercote
approved these changes
Oct 13, 2025
There was a problem hiding this comment.
Ha, I wrote almost exactly the same commit earlier today, but I hadn't uploaded it yet:
commit 0418d4a82fd4b675b61afa284a28d0c81196af95
Author: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Mon Oct 13 08:06:32 2025 +1100
Improve `ChunkedBitSet::clear`.
The current code overwrites the existing bitset with a new empty bitset,
which requires allocation. This PR changes it to just modify the
existing bitset without needing extra allocations. This makes `clear`
very similar to `insert_all`, which is nice. The new code is also
faster, though this doesn't really matter because `clear` isn't hot.
diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs
index f7649c5f5c5..7360c6f3fb8 100644
--- a/compiler/rustc_index/src/bit_set.rs
+++ b/compiler/rustc_index/src/bit_set.rs
@@ -584,9 +584,11 @@ pub fn new_filled(domain_size: usize) -> Self {
ChunkedBitSet::new(domain_size, /* is_empty */ false)
}
+ /// Sets all bits to false.
pub fn clear(&mut self) {
- let domain_size = self.domain_size();
- *self = ChunkedBitSet::new_empty(domain_size);
+ for chunk in self.chunks.iter_mut() {
+ *chunk = Zeros;
+ }
}
#[cfg(test)]
The only difference is I used a for loop.
r=me if you change clear to use a for loop or insert_all to use fill_with -- either way is fine, but it would be nice for them to look the same.
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
|
I don't think this needs a perf run. @bors r+ rollup |
bors
added
S-waiting-on-bors
Zalathar
removed
the
S-waiting-on-perf
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Oct 13, 2025
Clear `ChunkedBitSet` without reallocating There doesn't appear to be any reason to clear a ChunkedBitSet via its constructor (which allocates a new list of chunks), when we could just fill the existing allocation with `Chunk::Zeros` instead. For comparison, the `insert_all` impl added by the same PR (rust-lang#93984) does the simple thing here and just overwrites every chunk with `Chunk::Ones`. (That fill was then made somewhat easier by rust-lang#145480, which removes the chunk size from all-zero/all-one chunks.) r? nnethercote (or compiler)
This was referenced Oct 13, 2025
bors
added a commit
that referenced
this pull request
Oct 13, 2025
Rollup of 6 pull requests Successful merges: - #147514 (repr_transparent_external_private_fields: normalize types during traversal) - #147605 (Add doc links between `{integer}::from_str_radix` and `from_str`) - #147608 (cg_llvm: Use `LLVMDIBuilderCreateGlobalVariableExpression`) - #147623 (Clear `ChunkedBitSet` without reallocating) - #147625 (Add a warning when running tests with the GCC backend and debug assertions are enabled) - #147626 (Generalize configuring LLD as the default linker in bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Oct 13, 2025
Rollup merge of #147623 - Zalathar:clear-mixed, r=nnethercote Clear `ChunkedBitSet` without reallocating There doesn't appear to be any reason to clear a ChunkedBitSet via its constructor (which allocates a new list of chunks), when we could just fill the existing allocation with `Chunk::Zeros` instead. For comparison, the `insert_all` impl added by the same PR (#93984) does the simple thing here and just overwrites every chunk with `Chunk::Ones`. (That fill was then made somewhat easier by #145480, which removes the chunk size from all-zero/all-one chunks.) r? nnethercote (or compiler)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There doesn't appear to be any reason to clear a ChunkedBitSet via its constructor (which allocates a new list of chunks), when we could just fill the existing allocation with
Chunk::Zerosinstead.For comparison, the
insert_allimpl added by the same PR (#93984) does the simple thing here and just overwrites every chunk withChunk::Ones.(That fill was then made somewhat easier by #145480, which removes the chunk size from all-zero/all-one chunks.)
r? nnethercote (or compiler)